Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added temporary fix for channels-last-like tensors for Resize op #4979

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added temporary fix for channels-last-like tensors for Resize op #4979

wants to merge 2 commits into from

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Nov 23, 2021

Fixes #4880

Description:

  • Added temporary fix for channels-last-like tensors for Resize op

Benchmarks: code

  • Upsampling 64 -> 224, x2 | x3 speed up for PTH->DType->Resize (64->224) ->Norm
Torch version: 1.11.0.dev20211123+cu111
Torchvision version: 0.12.0a0
PIL version: 8.4.0

# Master
[----------- Benchmark reader+transformation -----------]
                                                |   Time 
1 threads: ----------------------------------------------
      PIL->Resize (64->224) ->Tensor->Norm      |  1410.6
      PIL->Tensor->Resize (64->224) ->Norm      |   847.1
      PTH->DType->Resize (64->224) ->Norm       |  1574.1
      JIT: PTH->DType->Resize (64->224) ->Norm  |  1502.3
6 threads: ----------------------------------------------
      PIL->Resize (64->224) ->Tensor->Norm      |  1085.2
      PIL->Tensor->Resize (64->224) ->Norm      |   610.7
      PTH->DType->Resize (64->224) ->Norm       |  1314.2
      JIT: PTH->DType->Resize (64->224) ->Norm  |  1225.7

# This PR
[----------- Benchmark reader+transformation -----------]
                                                |   Time 
1 threads: ----------------------------------------------
      PIL->Resize (64->224) ->Tensor->Norm      |  1408.1
      PIL->Tensor->Resize (64->224) ->Norm      |   843.9
      PTH->DType->Resize (64->224) ->Norm       |   706.1
      JIT: PTH->DType->Resize (64->224) ->Norm  |   616.0
6 threads: ----------------------------------------------
      PIL->Resize (64->224) ->Tensor->Norm      |  1096.2
      PIL->Tensor->Resize (64->224) ->Norm      |   606.2
      PTH->DType->Resize (64->224) ->Norm       |   462.8
      JIT: PTH->DType->Resize (64->224) ->Norm  |   367.7

Times are in microseconds (us).

Notations:

  • PIL->Resize (64->224) ->Tensor->Norm = pil_loader, [T.Resize((osize, osize)), T.ToTensor(), T.Normalize(mean, std)]
  • PTH->DType->Resize (64->224) ->Norm = torch_loader, [T.ConvertImageDtype(torch.float), T.Resize((osize, osize)), T.Normalize(mean, std)]
  1. Downsampling 500 -> 224, ~12% | ~25% speed up for PTH->DType->Resize (500->224) ->Norm
Torch version: 1.11.0.dev20211123+cu111
Torchvision version: 0.12.0a0
PIL version: 8.4.0

# Master
[---------- Benchmark reader+transformation -----------]
                                                 |  Time
1 threads: ---------------------------------------------
      PIL->Resize (500->224) ->Tensor->Norm      |  7.0 
      PIL->Tensor->Resize (500->224) ->Norm      |  7.9 
      PTH->DType->Resize (500->224) ->Norm       |  4.3 
      JIT: PTH->DType->Resize (500->224) ->Norm  |  4.2 
6 threads: ---------------------------------------------
      PIL->Resize (500->224) ->Tensor->Norm      |  7.1 
      PIL->Tensor->Resize (500->224) ->Norm      |  6.8 
      PTH->DType->Resize (500->224) ->Norm       |  3.8 
      JIT: PTH->DType->Resize (500->224) ->Norm  |  3.6 

# This PR
[---------- Benchmark reader+transformation -----------]
                                                 |  Time
1 threads: ---------------------------------------------
      PIL->Resize (500->224) ->Tensor->Norm      |  7.0 
      PIL->Tensor->Resize (500->224) ->Norm      |  7.9 
      PTH->DType->Resize (500->224) ->Norm       |  3.8 
      JIT: PTH->DType->Resize (500->224) ->Norm  |  3.7 
6 threads: ---------------------------------------------
      PIL->Resize (500->224) ->Tensor->Norm      |  7.0 
      PIL->Tensor->Resize (500->224) ->Norm      |  6.8 
      PTH->DType->Resize (500->224) ->Norm       |  3.0 
      JIT: PTH->DType->Resize (500->224) ->Norm  |  3.0

Times are in milliseconds (ms).

Notations:

  • PIL->Resize (500->224) ->Tensor->Norm = pil_loader, [T.Resize((osize, osize)), T.ToTensor(), T.Normalize(mean, std)]
  • PTH->DType->Resize (500->224) ->Norm = torch_loader, [T.ConvertImageDtype(torch.float), T.Resize((osize, osize)), T.Normalize(mean, std)]

PS: to avoid reverting this PR, let's wait for Francisco's review.

@vfdev-5 vfdev-5 requested a review from fmassa November 23, 2021 15:39
@facebook-github-bot
Copy link

facebook-github-bot commented Nov 23, 2021 •
edited
Loading

💊 CI failures summary and remediations

As of commit 93522e4 (more details on the Dr. CI page):

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build unittest_linux_cpu_py3.8 (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

/root/project/torchvision/io/video.py:399: Runt...log: [mov,mp4,m4a,3gp,3g2,mj2] moov atom not found 

test/test_internet.py::TestDatasetUtils::test_download_url_retry_http
  /root/project/env/lib/python3.8/email/feedparser.py:475: ResourceWarning: unclosed <ssl.SSLSocket [closed] fd=13, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6>
    for lineno, line in enumerate(lines):

test/test_internet.py::TestDatasetUtils::test_download_url_dispatch_download_from_google_drive
  /root/project/env/lib/python3.8/unittest/mock.py:2030: ResourceWarning: unclosed <ssl.SSLSocket [closed] fd=13, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6>
    setattr(_type, entry, MagicProxy(entry, self))

test/test_io.py::TestVideo::test_read_video_timestamps_corrupted_file
  /root/project/torchvision/io/video.py:399: RuntimeWarning: Failed to open container for /tmp/tmpjwjajg42.mp4; Caught error: [Errno 1094995529] Invalid data found when processing input: '/tmp/tmpjwjajg42.mp4'; last error log: [mov,mp4,m4a,3gp,3g2,mj2] moov atom not found
    warnings.warn(msg, RuntimeWarning)

test/test_models.py::test_memory_efficient_densenet[densenet121]
test/test_models.py::test_memory_efficient_densenet[densenet169]
test/test_models.py::test_memory_efficient_densenet[densenet201]
test/test_models.py::test_memory_efficient_densenet[densenet161]
  /root/project/env/lib/python3.8/site-packages/torch/autocast_mode.py:162: UserWarning: User provided device_type of 'cuda', but CUDA is not available. Disabling
    warnings.warn('User provided device_type of \'cuda\', but CUDA is not available. Disabling')

test/test_models.py::test_inception_v3_eval
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@vfdev-5
Added temporary fix for channels-last-like tensors for Resize op
93522e4 
Fixes pytorch#4880

Description:
- Added temporary fix for channels-last-like tensors for Resize op

Benchmarks:
@vfdev-5 vfdev-5 force-pushed the fix-4880-slow-resize branch from 76a3458 to 93522e4 Compare November 23, 2021 15:40
datumbox
datumbox reviewed Aug 22, 2022
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be in favour of merging this to resolve the speed issue until there are highly optimized kernels that handle channels last. This is related to pytorch/pytorch#83840 and #6465

cc @NicolasHug

@NicolasHug
Copy link
Member

NicolasHug commented Aug 22, 2022 •
edited
Loading

I think it's worth getting clarity on whether pytorch/pytorch#83840 is a fix-worthy issue first. Fixing pytorch/pytorch#83840 might make this PR obsolete.

This PR seems very similar to #4898 which was reverted from @fmassa 's #4898 (comment):

This was here on purpose for speed improvements on the reading (avoids copy).

I would revert this PR, and work on improving the performance on the transforms instead so that they better handle channel_last format

that still sounds applicable here?

@bossjones bossjones mentioned this pull request Jun 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datumbox datumbox datumbox left review comments

@fmassa fmassa Awaiting requested review from fmassa

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug with transforms.Resize when used with transforms.ConvertImageDtype
5 participants
@vfdev-5 @facebook-github-bot @NicolasHug @datumbox @pmeier